-
-
Notifications
You must be signed in to change notification settings - Fork 355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Batching fail fix #1048
Batching fail fix #1048
Conversation
f4a4cba
to
7c9550a
Compare
src/batchproof_container.cpp
Outdated
void BatchProofContainer::erase(std::vector<LelantusSigmaProofData>* vProofs, const Scalar& serial) { | ||
for (auto dataItr = vProofs->begin(); dataItr != vProofs->end(); dataItr++) { | ||
if (dataItr->serialNumber == serial) { | ||
vProofs->erase(dataItr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will crash if serial is the last element of vProofs
(dataItr
will be past the end of vector). Also std::vector<T>::erase
will invalidate iterators in some cases. I'd do it with std::remove_if
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree, done
The PR fixes batch verification failing issue in case of reindex/sync, The fail was happening when you had reorgs in chain, During DisconnectBlock it should remove proofs from container, but it was not removed and verification failed as proof from wrong chain is not valid after reorg.
Removing was not being done correctly as third element in key, was being set as true, 7c9550a#diff-755c76a1b8be7607bc371ca3a5b4ea39a91bf46946913fd5fd67b4d06b808d7dR97 ,
but actually it should be false, but now in removeLelantus() function is being removed also sigmaToLelantus proofs.